Support multiple authenticators with path matching by philipgough · Pull Request #52 · rhobs/api

philipgough · 2026-04-10T09:33:07Z

What

Although the proxy allowed an end user to configure multiple authenticators, at runtime, only one was selected in order of preference.

This change allows support for multiple authenticators.
The default behaviour remains intact.
Optionally, users can specify paths which should not match for specific authenticators.
It also adds middleware that ensures that at least one authenticator has run in order to support users not mistakingly bypassing auth with their configuration.

Furthermore, this change sets up an environment similar to what we have in RHOBS Cell in a KinD cluster.
This utility was badly needed in any case to facilitate the testing of auth which was practically non-existent or difficult to hook into.

It generates a simple configuration that splits read and write for logs and metrics on oidc for read and mtls for write respectively.

Key points to note that if you want optional mtls the api needs to be passed the - --tls.client-auth-type=RequestClientCert flag that already existed prior to this change.

From the working test, here is a sample of what a tenants config for multi-auth will look like

tenants:
- name: auth-tenant
  id: "1610702597"
  oidc:
    clientID: observatorium-api
    clientSecret: ZXhhbXBsZS1hcHAtc2VjcmV0
    issuerURL: http://dex.proxy.svc.cluster.local:5556/dex
    redirectURL: http://localhost:8080/oidc/auth-tenant/callback
    usernameClaim: email
    paths:
    - operator: "!~"
      pattern: ".*(loki/api/v1/push|api/v1/receive).*"
  mTLS:
    caPath: /etc/certs/ca.crt
    paths:
    - operator: "=~"
      pattern: ".*(api/v1/receive).*"
    - operator: "=~"
      pattern: ".*(loki/api/v1/push).*"

…nv to support multiple authenticators

…tching

philipgough · 2026-04-10T09:42:54Z

authentication/http.go

-// for the given list of tenants. Otherwise, it returns an error.
-// It protects the Prometheus remote write and Loki push endpoints. The tracing endpoint is not protected because
-// it goes through the gRPC middleware stack, which behaves differently from the HTTP one.
-func EnforceAccessTokenPresentOnSignalWrite(oidcTenants map[string]struct{}) func(http.Handler) http.Handler {


i really dont understand why this was here. it was enforcing oidc on all write requests.

I suppose it ignores it further down, but good refactor. Your implementation is clearer.

moadz

How do we protect ourselves from accidentally declaring overlapping paths on the middlewares for a particular tenant / path. If they overlap how do we determine priority, and if they do overlap and one fails the other will not trigger. The order by which we add them is also non-determinisitc in main.go@640.

We could write a test for the above ^^ (forgive me if i don't see it)

Other than that looks good, thanks for figuring this out. :)

moadz · 2026-04-13T08:37:42Z

authentication/authentication.go

 func (pm *ProviderManager) GRPCMiddlewares(tenant string) (grpc.StreamServerInterceptor, bool) {
 	pm.mtx.RLock()
-	mw, ok := pm.gRPCInterceptors[tenant]
+	interceptors, ok := pm.gRPCInterceptors[tenant]


😗👌🏽

moadz · 2026-04-13T08:38:45Z

authentication/authentication.go

+	}
+
+	// If only one interceptor, return it directly
+	if len(interceptors) == 1 {


Is this conditional needed? We return interceptors[0], true regardless at the end of this method.

it was just for consistency since this doesnt really work here right now and to keep the logic the same, but happy to adapt

moadz · 2026-04-13T08:40:25Z

authentication/authentication.go

+		return interceptors[0], true
+	}
+
+	// Compose multiple interceptors into because this is production code, id like an actual wort (simplified approach for now)


moadz · 2026-04-13T08:44:09Z

authentication/http.go

-// for the given list of tenants. Otherwise, it returns an error.
-// It protects the Prometheus remote write and Loki push endpoints. The tracing endpoint is not protected because
-// it goes through the gRPC middleware stack, which behaves differently from the HTTP one.
-func EnforceAccessTokenPresentOnSignalWrite(oidcTenants map[string]struct{}) func(http.Handler) http.Handler {


I suppose it ignores it further down, but good refactor. Your implementation is clearer.

moadz · 2026-04-13T08:48:11Z

authentication/mtls.go

+		if operator == "" {
+			operator = OperatorMatches
+		}


I haven't got to that part of the PR, but we should document that the default behaviour flag docs is matching if we include this handling.

moadz · 2026-04-13T08:53:51Z

authentication/mtls_test.go

-			pathPatterns: []string{"^/api/metrics/.*/(receive|rules)$"},
+			pathPatterns: []PathPattern{
+				{Operator: "=~", Pattern: "^/api/metrics/.*/(receive|rules)$"},
+			},


afaics none of the tests cover the non-matching regex path (i.e. OperatorNotMatches path pattern matching operation.

we have coverage in the new e2e file in test/kind e2e.go

moadz · 2026-04-13T09:00:14Z

main.go

Dead func right?

moadz · 2026-04-13T09:03:29Z

authentication/mtls.go

+			operator = OperatorMatches
+		}
+
+		if operator != OperatorMatches && operator != OperatorNotMatches {


I wonder if having both isn't redundant. I.e. if a user specifies [{!~, /query}, {=~, /receive} the former is enforced and the latter is ignored. imho we should bias to explicit matching only.

That was my first iteration, but because go does not support negative lookaheads and only RE2 it is going to make configuration extremely complicate to avoid overlpas if one so wishes.

For example, take a look at

oidc: clientID: observatorium-api clientSecret: ZXhhbXBsZS1hcHAtc2VjcmV0 issuerURL: http://dex.proxy.svc.cluster.local:5556/dex redirectURL: http://localhost:8080/oidc/auth-tenant/callback usernameClaim: email paths: - operator: "!~" pattern: ".*(loki/api/v1/push|api/v1/receive).*" mTLS: caPath: /etc/certs/ca.crt paths: - operator: "=~" pattern: ".*(api/v1/receive).*" - operator: "=~" pattern: ".*(loki/api/v1/push).*"

and imagine the alternative where the operator isnt available.

I would have preferred the original too for simplicity but can confidently say having iterated through the test environment to try to achieve the outcome above, this is much better and leads to much simpler runtime config for end user.

philipgough · 2026-04-13T09:25:47Z

How do we protect ourselves from accidentally declaring overlapping paths on the middlewares for a particular tenant

We dont, and thats by design. If the specify a regex that will match we run the authenticator. I think this makes the most sense as it allows us to combine authenticators if desired (it could make sense to support mtls and oidc/opa for example).

Its also impossible almost to do otherwise because of the nature of regex. Verifying no overlap would be extremely complicated if at all possible. We would document this as an advanced feature but to be honest, the main priority is that some auth does run and ive added middleware to ensure that must happen and that paths are not accidentally exposed.

If they overlap how do we determine priority, and if they do overlap and one fails the other will not trigger.

Again this is by design. If you have configured an overlap then they all run, if one fails its a hard fail

philipgough added 6 commits April 10, 2026 10:30

Add support for path based auth

d96d7ac

e2e: Adds an environment on a kind cluster to replicate a real-life e…

1e32b0c

…nv to support multiple authenticators

Support like and not like operators on path regex to simplify path ma…

06979c6

…tching

Fix issue with path matching logic

50f0e31

Multi authenticators

15178ad

Adds arch diagram for test env in kind

b426d9d

philipgough force-pushed the multi-auth-safe branch from 9772bad to b426d9d Compare April 10, 2026 09:39

philipgough commented Apr 10, 2026

View reviewed changes

moadz approved these changes Apr 13, 2026

View reviewed changes

make lint

79eeca0

philipgough force-pushed the multi-auth-safe branch from 65d67fb to 79eeca0 Compare April 13, 2026 11:20

philipgough merged commit d2e2a61 into rhobs-obs-api-konflux Apr 14, 2026

Conversation

philipgough commented Apr 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moadz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philipgough commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants